compiler: bound aggregate call signatures - #5625
Draft
jakebailey wants to merge 2 commits into
Draft
Conversation
Plan internal aggregate parameter lowering from each complete function signature. Count scalar leaves, the context parameter, and any hidden aggregate-result pointer, then pass the largest aggregates indirectly until the signature fits within the 1,000-parameter limit. This keeps the ABI policy target-independent and leaves fitting signatures unchanged. Preserve exported ABIs, validate final WebAssembly signatures that cannot be rewritten, release temporary roots before final dead-code elimination, and diagnose incompatible exported methods.
jakebailey
commented
Aug 29, 2026
There was a problem hiding this comment.
Pull request overview
This PR limits scalarized aggregate function signatures to 1,000 parameters.
Changes:
- Selects aggregate parameters for indirect passing.
- Handles calls, interfaces, defers, goroutines, and exported ABI validation.
- Adds ABI and global-reference tests.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
builder/build.go |
Validates WebAssembly signatures. |
compiler/calls.go |
Centralizes ABI argument handling. |
compiler/compiler.go |
Applies ABI choices to calls and functions. |
compiler/compiler_test.go |
Tests aggregate ABI limits. |
compiler/defer.go |
Applies the ABI to deferred calls. |
compiler/func.go |
Calculates and validates function ABIs. |
compiler/goroutine.go |
Applies the ABI to goroutine arguments. |
compiler/interface.go |
Handles interface invoke wrappers. |
compiler/llvmutil/llvm.go |
Removes temporary global references. |
compiler/llvmutil/llvm_test.go |
Tests global-reference removal. |
compiler/symbol.go |
Protects indirect signatures during optimization. |
compiler/testdata/aggregate-abi.go |
Adds internal ABI fixtures. |
compiler/testdata/aggregate-export-abi.go |
Adds exported ABI fixtures. |
transform/interface-lowering.go |
Reports incompatible interface ABIs. |
transform/optimizer.go |
Removes temporary ABI roots. |
Suppressed comments (2)
compiler/compiler.go:2326
- This ABI calculation omits the synthetic typecode parameter that is appended below. At the 1000 parameter boundary, the invoke function spills an aggregate but this call passes it directly, which gives the callee a different ABI.
abi = b.getInterfaceFunctionABI(instr.Signature())
compiler/interface.go:1350
- This uses the concrete receiver when it selects indirect parameters. The interface invoke ABI replaces that receiver with a pointer, so it can select different nonreceiver parameters and the wrapper then forwards values with the wrong ABI.
receiverType := abi.params[0].llvmType
var expandedReceiverType []llvm.Type
receiverIndirect := abi.params[0].indirect
var receiverInfos []paramInfo
if receiverIndirect {
receiverInfos = []paramInfo{{llvmType: c.dataPtrType}}
} else {
receiverInfos = c.expandDirectFormalParamType(receiverType, "", nil)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Include the interface typecode when budgeting invoke signatures and keep method parameter lowering consistent between concrete and interface calls. Preserve valid exported methods with large receivers. Materialize indirect aggregate phi inputs in their predecessor blocks, and retain temporary argument-promotion guards through the ThinLTO pre-link pipeline before final dead-code elimination.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
compiler/compiler_test.go:249
- This test checks only that the compiler emits the marker. It does not run
LowerInterfaces, so the new diagnostic path is not tested. Add a transform or build test that checks the expected error and a compatible exported interface call.
if markedWrappers != 1 {
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5615
This is a sort-of more involved alternative to #5618; the gist is that we ensure we limit everything to up to 1000 scalar elements across the board. Back when I did #5526, I had something like this, but scaled it back because I didn't think exactness would matter too much so long as we didn't overload LLVM. But, I guess it does matter.